Skip to content

fix: unified restart required#1978

Merged
yinwm merged 5 commits intosipeed:mainfrom
SiYue-ZO:feature/unified-restart-required
Mar 28, 2026
Merged

fix: unified restart required#1978
yinwm merged 5 commits intosipeed:mainfrom
SiYue-ZO:feature/unified-restart-required

Conversation

@SiYue-ZO
Copy link
Copy Markdown
Contributor

@SiYue-ZO SiYue-ZO commented Mar 24, 2026

📝 Description

Unify the restart-required detection and notification mechanism so that model, tool, and configuration changes all follow the same logic, preventing user confusion.

Key Changes:

  1. Backend: Unified Restart Detection

    • Add bootConfigSignature to track restart-sensitive configuration at gateway startup
    • Implement computeConfigSignature() to capture default model and tool enabled states
    • Add gatewayRestartRequiredBySignature() for unified restart detection
    • Update gatewayStatusData() to use signature-based comparison
  2. Frontend: State Synchronization

    • ToolsPage: Refresh gateway state after tool toggle success
    • ConfigPage: Refresh gateway state after visual config save success
    • RawConfigPage: Refresh gateway state after raw JSON save success
  3. Copy Unification

    • Update restart-required message from model-specific to generic config change
    • Sync Chinese and English localization

🗣️ Type of Change

  • 🐞 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 📖 Documentation update
  • ⚡ Code refactoring (no functional changes, no api changes)

🤖 AI Code Generation

  • 🤖 Fully AI-generated (100% AI, 0% Human)
  • 🛠️ Mostly AI-generated (AI draft, Human verified/modified)
  • 👨‍💻 Mostly Human-written (Human lead, AI assisted or none)

🔗 Related Issue

Fixes inconsistent restart-required notification behavior across model, tool, and configuration changes.

📚 Technical Context (Skip for Docs)

  • Reference URL: N/A
  • Reasoning: The original implementation only detected default model changes. Tool toggles and configuration changes did not trigger restart notifications. By introducing a configuration signature mechanism, all restart-sensitive changes are now uniformly detected.

🧪 Test Environment

  • Hardware: PC
  • OS: Linux
  • Model/Provider: N/A
  • Channels: N/A

📸 Evidence (Optional)

Click to view Test Results
=== RUN   TestGatewayStatusRequiresRestartAfterDefaultModelChange
--- PASS: TestGatewayStatusRequiresRestartAfterDefaultModelChange (0.01s)
=== RUN   TestGatewayStatusRequiresRestartAfterToolChange
--- PASS: TestGatewayStatusRequiresRestartAfterToolChange (0.01s)
=== RUN   TestGatewayStatusNoRestartRequiredForNonSensitiveChanges
--- PASS: TestGatewayStatusNoRestartRequiredForNonSensitiveChanges (0.01s)
=== RUN   TestGatewayStatusNoRestartRequiredWhenNotRunning
--- PASS: TestGatewayStatusNoRestartRequiredWhenNotRunning (0.01s)
PASS
ok      github.com/sipeed/picoclaw/web/backend/api      0.043s

☑️ Checklist

  • My code/docs follow the style of this project.
  • I have performed a self-review of my own changes.
  • I have updated the documentation accordingly.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 24, 2026

CLA assistant check
All committers have signed the CLA.

@SiYue-ZO
Copy link
Copy Markdown
Contributor Author

SiYue-ZO commented Mar 24, 2026

Closes #1973

@SiYue-ZO SiYue-ZO changed the title Feature/unified restart required fix: unified restart required Mar 25, 2026
@SiYue-ZO SiYue-ZO force-pushed the feature/unified-restart-required branch from 9d1d75d to 297a6c6 Compare March 25, 2026 04:20
@yinwm
Copy link
Copy Markdown
Collaborator

yinwm commented Mar 25, 2026

plz fix Linter @SiYue-ZO

- Add bootConfigSignature to gateway state for tracking restart-sensitive config
- Implement computeConfigSignature() to capture model and tool enabled states
- Add gatewayRestartRequiredBySignature() for unified restart detection
- Update gatewayStatusData() to use signature-based comparison
- Preserve backward compatibility with existing boot_default_model field
- TestGatewayStatusRequiresRestartAfterToolChange: verify tool toggle triggers restart hint
- TestGatewayStatusNoRestartRequiredForNonSensitiveChanges: verify non-sensitive changes don't trigger
- TestGatewayStatusNoRestartRequiredWhenNotRunning: verify no restart hint when gateway stopped
- Update resetGatewayTestState to clear bootConfigSignature
- ToolsPage: call refreshGatewayState after tool toggle success
- ConfigPage: call refreshGatewayState after config save success
- RawConfigPage: call refreshGatewayState after raw JSON save success
- Ensures restart-required indicator updates immediately after changes
- Update restartRequired message from model-specific to generic config change
- zh: '切换默认模型后需要重启服务才能生效' -> '配置变更后需要重启服务才能生效'
- en: 'Model changes require a gateway restart' -> 'Configuration changes require a gateway restart'
- Reflects unified restart detection for model/tools/config changes
@SiYue-ZO
Copy link
Copy Markdown
Contributor Author

I will fix it

@SiYue-ZO SiYue-ZO force-pushed the feature/unified-restart-required branch from 8de0be6 to 0858b16 Compare March 25, 2026 10:52
@SiYue-ZO
Copy link
Copy Markdown
Contributor Author

I've fixed Linter @yinwm

Copy link
Copy Markdown
Collaborator

@yinwm yinwm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The signature-based approach cleanly extends the restart-required detection from model-only to model+tools+config. Good test coverage with the 3 new test cases. Frontend changes are minimal and correct.

Non-blocking note: consider adding a comment above computeConfigSignature() reminding maintainers to update the tool list when new tools are added.

@yinwm yinwm merged commit 27f638e into sipeed:main Mar 28, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants